Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(base) Give a hint in web interface if SimpleXML (php-xml) is missing #38857

Merged

Conversation

joshtrichards
Copy link
Member

@joshtrichards joshtrichards commented Jun 16, 2023

Summary

Avoids a 500 error and also gives a useful error message on the web interface if this module isn't installed, gets overlooked during a PHP upgrade, etc.

TODO

  • Maybe some field testing?

Checklist

@szaimen szaimen added the 3. to review Waiting for reviews label Jun 16, 2023
@szaimen szaimen added this to the Nextcloud 28 milestone Jun 16, 2023
@szaimen szaimen requested review from a team, ArtificialOwl, nfebe and come-nc and removed request for a team June 16, 2023 13:59
@joshtrichards joshtrichards force-pushed the jr-earlier-check-for-php-xml branch from a090776 to 856967a Compare June 16, 2023 15:02
@joshtrichards
Copy link
Member Author

Test failures appear unrelated. jsunit had a bunch of setup errors.

Copy link
Contributor

@come-nc come-nc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How will cli behave in this situation?

@joshtrichards
Copy link
Member Author

The CLI will behave how it does now - i.e. it'll generate an Error: Call to undefined function OC\App\simplexml_load_file() in... stack trace.

I suppose this is an opportunity to provide a friendly message at the command-line too, but the stack trace is at least visible there...

lib/base.php Outdated Show resolved Hide resolved
@skjnldsv skjnldsv mentioned this pull request Nov 1, 2023
This was referenced Nov 6, 2023
This was referenced Nov 14, 2023
@blizzz blizzz modified the milestones: Nextcloud 28, Nextcloud 29 Nov 23, 2023
@AndyScherzinger AndyScherzinger force-pushed the jr-earlier-check-for-php-xml branch from 856967a to 14c2b40 Compare February 27, 2024 13:23
lib/base.php Fixed Show fixed Hide fixed
This was referenced Mar 12, 2024
This was referenced Mar 20, 2024
@skjnldsv skjnldsv mentioned this pull request Mar 28, 2024
81 tasks
@skjnldsv skjnldsv modified the milestones: Nextcloud 29, Nextcloud 30 Mar 28, 2024
@skjnldsv skjnldsv added the 2. developing Work in progress label Jul 27, 2024
@skjnldsv skjnldsv added stale Ticket or PR with no recent activity and removed 3. to review Waiting for reviews labels Jul 27, 2024
This was referenced Jul 30, 2024
This was referenced Aug 5, 2024
@skjnldsv skjnldsv mentioned this pull request Aug 13, 2024
@skjnldsv skjnldsv closed this Aug 14, 2024
@skjnldsv skjnldsv removed this from the Nextcloud 30 milestone Aug 14, 2024
Fixes nextcloud#2180
And prevents nextcloud#31473, nextcloud#23970, nextcloud#18610, nextcloud#15708

Avoids a 500 error and also gives a useful error message on the web interface if this module isn't installed, gets overlooked during a PHP upgrade, etc.

While we check for it later, it's too late for session.

Inspired by nextcloud#17163

Signed-off-by: Josh Richards <[email protected]>
@skjnldsv skjnldsv reopened this Aug 16, 2024
@skjnldsv skjnldsv force-pushed the jr-earlier-check-for-php-xml branch from 14c2b40 to 8032b6a Compare August 16, 2024 09:06
@skjnldsv
Copy link
Member

Rebased and addressed 💪

@skjnldsv skjnldsv added bug 3. to review Waiting for reviews 4. to release Ready to be released and/or waiting for tests to finish and removed 2. developing Work in progress stale Ticket or PR with no recent activity 3. to review Waiting for reviews labels Aug 16, 2024
@skjnldsv skjnldsv merged commit b8b8125 into nextcloud:master Aug 16, 2024
155 of 161 checks passed
@skjnldsv
Copy link
Member

🎉 🎉 🎉

@skjnldsv skjnldsv added this to the Nextcloud 31 milestone Aug 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PHP7 Check for missing modules incomplete (should check for php-xml)
6 participants